Skip to content

make pallet evm dispatches call, create public#358

Closed
peektism wants to merge 5 commits intopolkadot-evm:masterfrom
peektism:make-evm-call-public
Closed

make pallet evm dispatches call, create public#358
peektism wants to merge 5 commits intopolkadot-evm:masterfrom
peektism:make-evm-call-public

Conversation

@peektism
Copy link
Copy Markdown
Contributor

This makes testing easier for moonbeam-foundation/moonbeam#339 and I'm unaware of negative consequences of making this change. If there is a reason this shouldn't be done, I want to understand why.

@peektism peektism requested a review from sorpaas as a code owner April 14, 2021 15:50
Comment thread frame/evm/src/lib.rs
Copy link
Copy Markdown
Member

@sorpaas sorpaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please also change pallet-evm version to 3.0.1-dev and recursively all upper crates before merging!

sp-io = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
frame-support = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
pallet-evm = { version = "3.0.0", default-features = false, path = "../.." }
pallet-evm = { version = "3.0.1-dev", default-features = false, path = "../.." }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any bump of dependency should be recursive. See https://github.com/paritytech/frontier#versioning

Alternatively, since it's a patch and no interface is changing, you can also just revert those dependency bumps.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I reverted the dependency bumps. There are no interface changes so I think this is appropriate.

@peektism peektism changed the title make pallet evm call runtime method public make pallet evm dispatches call, create public Apr 14, 2021
Comment thread frame/evm/Cargo.toml Outdated
[package]
name = "pallet-evm"
version = "3.0.1-dev"
version = "3.0.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You at least need to have this!

@peektism
Copy link
Copy Markdown
Contributor Author

I'm going to try something else to fix my problem and if it doesn't work, I'll come back to this and do the recursive bump that you mentioned earlier.

@peektism
Copy link
Copy Markdown
Contributor Author

The thing I tried worked, I don't need this change.

@peektism peektism closed this Apr 14, 2021
@peektism peektism deleted the make-evm-call-public branch April 14, 2021 18:07
@JoshOrndorff

This comment has been minimized.

@peektism
Copy link
Copy Markdown
Contributor Author

@girazoki showed me this https://github.com/PureStake/crowdloan-rewards/blob/main/src/tests.rs#L290 which enables calling private runtime methods

@JoshOrndorff
Copy link
Copy Markdown
Contributor

JoshOrndorff commented Apr 14, 2021

Indeed proxying through pallet_utility::batch is an effective workaround for Moonbeam. This will still be needed for testing any runtime that doesn't use pallet_utility. And it makes the tests more readable even if utility is around. Maybe we can re-open it when time permits.

(And we should do create2 at that time also)

@peektism
Copy link
Copy Markdown
Contributor Author

peektism commented Apr 15, 2021

I meant that I'm using the same pattern to make the private call. It declares a Call type generic over the Runtime which is able to form the call correctly. Here is the code but the call still fails and I haven't been able to figure out why moonbeam-foundation/moonbeam@97fc054#diff-4a6734b9be4e30f208c6afca6c81c41fca067292e6e5aef519e8cb0c8be84122R204

So this pattern enables us to call private runtime methods without making them public. This means we don't need to make these methods public for testing as far as I understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants